Skip to content

Optimize TerminalCommandDisplay visual wrapping with early-exit preview - #1224

Open
nordicnode wants to merge 2 commits into
CodebuffAI:mainfrom
nordicnode:perf-terminal-command-display-wrapping
Open

Optimize TerminalCommandDisplay visual wrapping with early-exit preview#1224
nordicnode wants to merge 2 commits into
CodebuffAI:mainfrom
nordicnode:perf-terminal-command-display-wrapping

Conversation

@nordicnode

@nordicnode nordicnode commented Sep 2, 2026

Copy link
Copy Markdown

Summary

  • In cli/src/components/terminal-command-display.tsx, optimize <TerminalCommandDisplay /> to compute visual line wrapping on-demand and exit early once the preview limit (maxLines) is reached.
  • Early-exit line wrapping: In the collapsed state (!isExpanded), breaks out of line wrapping as soon as maxLines visual lines are gathered, evaluating only the lines needed for the preview rather than running the regex word-splitter across all lines.
  • Blank line preservation: Treats blank lines as 1 visual line across preview gathering (displayLines.push('')), off-screen visual line counts, and expanded line checks, ensuring 100% layout parity for command outputs with interstitial blank lines (such as build logs, test summaries, and git log --stat).
  • Accurate partial-line excess tracking: Tracks the exact number of visual lines remaining when a multi-line wrapped line is partially cut off at the preview boundary (excessInProcessedLine = wrapped.length - i).
  • Hybrid off-screen line counting: For off-screen lines, computes exact visual line counts using getLastNVisualLines on the first 50 off-screen lines, and uses a fast character-width estimate Math.max(1, Math.ceil(line.length / width)) for lines beyond 50. This provides exact line counts on common command outputs while preventing CPU stalls on massive (e.g. 10,000-line) logs.
  • Expanded rendering: When expanded (isExpanded), full output is directly passed to the OpenTUI text renderer without intermediate wrapping allocations, checking hasMoreLines in O(1).
  • Dedicated OpenTUI tests: In cli/src/components/__tests__/terminal-command-display.test.tsx, added component tests verifying:
    • Short output without truncation.
    • Output exceeding maxVisibleLines with "Show more" button.
    • Long wrapped single line wrapping.
    • Preservation of interstitial blank lines in preview.
    • Accurate hidden line counts with blank lines in off-screen content.
    • Accurate line counts when a wrapped line crosses the preview boundary.

Test plan

  • bun test --config=/dev/null src/components/__tests__/terminal-command-display.test.tsx (6 passed, 0 failed)
  • bun test --config=/dev/null src/components/tools/__tests__/run-terminal-command.test.ts (14 passed, 0 failed)
  • bun run --cwd cli typecheck (0 errors)
  • bun freebuff/cli/build.ts 0.0.0-ci (successful binary build)
  • Binary smoke test: bun cli/scripts/smoke-binary.ts cli/bin/freebuff (OK)
  • Prettier check: bun x prettier --check cli/src/components/terminal-command-display.tsx cli/src/components/__tests__/terminal-command-display.test.tsx (All matched files use Prettier code style)

@codebuff-team

Copy link
Copy Markdown
Contributor

Nice motivation and benchmarking — early-exiting the wrap loop once maxLines is reached is the right idea for terminal-command-display.tsx, and the OpenTUI render tests are a welcome addition since this component previously had none.

However, there's a real correctness bug in the collapsed-state loop:

if (line.length === 0) {
  linesProcessed++
  continue
}

This skips blank lines entirely instead of treating them as one visual line (as getLastNVisualLines did originally). Any command output with interstitial blank lines (build logs, test summaries, git log --stat, etc.) will render a preview missing those blank lines — a visible layout change, not just a perf win. The same skip happens in the remainingVisualLines estimate and the isExpanded totalVisual calc, so hiddenLinesCount/hasMoreLines will also be off whenever blank lines are present. None of the three new tests exercise output with blank lines, so this slipped through despite the '100% exact parity' claim in the PR body.

Separately, the off-screen line-count estimate (Math.ceil(line.length / width)) is a character-count approximation of what the regex-based word wrapper actually produces. That's a reasonable perf trade-off for a hidden-line counter, but it means the 'Show N more lines' label can now be wrong by a line or two versus the real wrap result — worth calling out explicitly as an approximation rather than presenting it as parity-preserving.

Fix the blank-line handling (count it as one line, don't skip it) and either compute the hidden count from the real wrapper for the first chunk past maxLines or document the approximation, and this becomes a solid port candidate.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written labels Sep 3, 2026
@nordicnode

Copy link
Copy Markdown
Author

Thanks for catching this @codebuff-team! Both points have been resolved in the latest commit:

  1. Blank Line Preservation & Counting: Blank lines are now counted as 1 visual line across all three paths:
    • In the collapsed preview loop: if (line.length === 0) now pushes '' to displayLines, preserving blank lines in the rendered output rather than skipping them, and breaks once maxLines visual lines are collected.
    • In off-screen line counting: blank lines increment remainingVisualLines++.
    • In isExpanded: blank lines increment totalVisual++.
    • In addition, partial wrapped-line excess is now tracked precisely (excessInProcessedLine = wrapped.length - i) when a wrapped line is truncated at the boundary.
  2. Hybrid Off-Screen Line Counting: For off-screen lines, the component now uses the exact getLastNVisualLines wrapper on the first 50 off-screen lines, and applies the fast character-width estimate Math.max(1, Math.ceil(line.length / width)) for lines beyond 50. This gives exact line counts on common command outputs while preserving the performance advantage on massive (e.g. 10,000-line) logs. We've also documented this approximation in the PR summary.
  3. OpenTUI Test Suite: Added test cases in cli/src/components/__tests__/terminal-command-display.test.tsx verifying:
    • Interstitial blank lines in preview are preserved and counted toward maxVisibleLines.
    • Off-screen blank lines are accurately counted in hiddenLinesCount.
    • Multi-line wrapped line boundary truncation is counted accurately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:needs-work Right idea, not mergeable as written

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants